Skip to content

fix: standardize sandbox data api errors#95

Open
TJKkking wants to merge 3 commits intomainfrom
dev-sb-responce
Open

fix: standardize sandbox data api errors#95
TJKkking wants to merge 3 commits intomainfrom
dev-sb-responce

Conversation

@TJKkking
Copy link
Copy Markdown
Collaborator

@TJKkking TJKkking commented Apr 30, 2026 •
edited
Loading

Raise ClientError/ServerError for Sandbox HTTP JSON error responses and expose structured error metadata on HTTPError. Document the breaking migration in release notes.

Tests: uv run pytest tests/unittests/utils/test_exception.py tests/unittests/sandbox/api/test_sandbox_data.py; uv run pytest tests/unittests/sandbox/api/test_code_interpreter_data.py tests/unittests/sandbox/api/test_browser_data.py tests/unittests/sandbox/api/test_aio_data.py tests/unittests/sandbox/test_client.py.

Type check: targeted mypy passed for modified files. Full mypy is blocked by existing duplicate module sandbox from local/sandbox/init.py and examples/sandbox.py.

Thank you for creating a pull request to contribute to Serverless Devs agentrun-sdk-python code! Before you open the request please answer the following questions to help it be more easily integrated. Please check the boxes "[ ]" with "[x]" when done too.
Please select one of the PR types below to complete

Fix bugs

Bug detail

The specific manifestation of the bug or the associated issue.

Pull request tasks

  • Add test cases for the changes
  • Passed the CI test

Update docs

Reason for update

Why do you need to update your documentation?

Pull request tasks

  • Update Chinese documentation
  • Update English documentation

Add contributor

Contributed content

  • Code
  • Document

Content detail

if content_type == 'code' || content_type == 'document':
    please tell us `PR url`，like: https://github.com/Serverless-Devs/agentrun-sdk-python/pull/1
else:
    please describe your contribution in detail

Others

Reason for update

Why do you need to update your documentation?

@TJKkking
Copy link
Copy Markdown
Collaborator Author

TJKkking commented Apr 30, 2026

@claude code review

@TJKkking
fix: standardize sandbox data api errors
1f291b5 
Raise ClientError/ServerError for Sandbox HTTP JSON error responses and expose structured error metadata on HTTPError. Document the breaking migration in release notes.

Tests: uv run pytest tests/unittests/utils/test_exception.py tests/unittests/sandbox/api/test_sandbox_data.py; uv run pytest tests/unittests/sandbox/api/test_code_interpreter_data.py tests/unittests/sandbox/api/test_browser_data.py tests/unittests/sandbox/api/test_aio_data.py tests/unittests/sandbox/test_client.py.

Type check: targeted mypy passed for modified files. Full mypy is blocked by existing duplicate module sandbox from local/sandbox/__init__.py and examples/sandbox.py.

Signed-off-by: 寒光 <2510399607@qq.com>
@TJKkking
test(unittests): Add an automatic fixture for test environment variab…
887c087 
…le isolation

- Added an `autouse` fixture to automatically clean up environment variables related to the SDK configuration.
- Prevents local `.env` files from interfering with the environment variable settings used in unit tests.
- Cleans up specific environment variables to prevent assertion failures within `respx` mocks.
- Ensures environment variable isolation between test cases, thereby enhancing test stability.
- Cleans up only those environment variables that the SDK reads by default, without affecting user-defined variables.

Signed-off-by: 寒光 <2510399607@qq.com>
@TJKkking TJKkking requested a review from Copilot April 30, 2026 05:35
Copilot AI reviewed Apr 30, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Standardizes Sandbox data-plane HTTP JSON API error handling so callers receive consistent ClientError/ServerError exceptions (with structured metadata) instead of needing to inspect returned dictionaries.

Changes:

  • Extend HTTPError to carry error_code, response_body, and response_headers, and update string formatting to avoid expanding response metadata.
  • Update SandboxDataAPI HTTP handling to raise ClientError for 4xx and ServerError for 5xx JSON error responses (sync + async), with robust request-id extraction.
  • Add unit tests for the new exception fields/formatting and Sandbox data-plane HTTP behaviors; add release notes documenting the breaking migration.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/unittests/utils/test_exception.py Adds coverage for new HTTPError metadata fields and updated __str__ behavior.
tests/unittests/sandbox/api/test_sandbox_data.py Adds respx-based tests validating sandbox data-plane success/error behaviors for sync/async calls.
tests/unittests/conftest.py Adds autouse env isolation to prevent local .env from polluting unit tests.
agentrun/utils/exception.py Adds structured error metadata to HTTPError and updates formatting; simplifies ClientError/ServerError.
agentrun/sandbox/api/sandbox_data.py Implements standardized HTTP JSON error raising and structured error extraction for Sandbox data-plane requests.
agentrun/sandbox/api/__sandbox_data_async_template.py Updates codegen template to match the new sandbox data-plane HTTP error handling behavior.
RELEASE_NOTES.md Documents the breaking behavior change and migration guidance.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@OhYee
Copy link
Copy Markdown
Member

OhYee commented Apr 30, 2026

@copilot 检查下是否存在隐患，是否存在不兼容变动

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 30, 2026

@copilot 检查下是否存在隐患，是否存在不兼容变动

以下是变更中存在的隐患及不兼容点分析：

已记录的 Breaking Change（在 RELEASE_NOTES.md 中有说明）

  • Sandbox 数据层 HTTP 错误处理：SandboxDataAPI._make_request / _make_request_async 现在对 4xx/5xx 响应抛出 ClientError/ServerError，而非原来返回包含错误信息的 dict。影响 SandboxDataAPI 的所有公开方法（check_health、create_sandbox、delete_sandbox 等），任何通过检查返回 dict 中 code/requestId 来判断错误的代码均需按 RELEASE_NOTES.md 中的迁移指引更新。

未记录的 Breaking Change

  • HTTPError.__str__ 输出格式变更：旧格式固定为 "HTTP {status}: {msg}. Request ID: None. Details: {}" （无论字段是否为 None 都输出）；新格式仅在字段非空时才追加，且各部分之间改用 ". " 拼接。任何解析该字符串的代码（如日志处理、测试断言）将受到影响，但此变更未在 RELEASE_NOTES.md 中提及。

潜在风险（功能不一致）

  • 2xx 非 JSON 响应的异常类型变更：父类 DataAPI._parse_success_response 对非 JSON 的 2xx 响应抛出 ClientError，而新的 SandboxDataAPI._parse_success_response 抛出 ServerError。两者行为不一致，且沙箱端实际上并无过错，理论上 ClientError 更语义准确（解析失败属于可预期情况）。目前 CI 测试全部通过（4/4 runs 均 success），对实际用户影响较低。

向后兼容改动（无风险）

  • HTTPError 新增了 error_code、response_body、response_headers 字段（默认值为 None），现有调用方不受影响。
  • ServerError 移除了显式 __init__（原本不接受 **kwargs），现通过继承支持新字段，属于放宽约束，不影响现有调用。
  • 非 Sandbox 的 API 客户端（AgentRuntime、Model 等）均未受影响，因为它们未覆盖 _make_request。

建议：在 RELEASE_NOTES.md 中补充 HTTPError.__str__ 格式变更的说明，并将 _parse_success_response 中的 ServerError 改为 ClientError，以保持与父类行为一致。

Copilot AI requested a review from OhYee April 30, 2026 07:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@OhYee OhYee Awaiting requested review from OhYee

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@TJKkking @OhYee